-
Notifications
You must be signed in to change notification settings - Fork 8
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add Browser#openAndWait for convenience(Fixes #28) #30
Add Browser#openAndWait for convenience(Fixes #28) #30
Conversation
…our currently two minute duration
I don't like the 2-minute baked in wait time. I would just add the duration as an argument. You could have two overrides that follow the similar pattern as What we really probably want is this to be implemented: darcy-framework/synq#17 , as this would be the right way to do default wait times (as opposed to just assuming two minutes, which is really probably much too long... that we tend to have settled on that as a common default internally shouldn't really have a baring in the framework itself). Then you could just |
*/ | ||
default <T extends View> T openAndWait(ViewUrl<T> viewUrl) { | ||
return open(viewUrl).waitUpTo(Duration.of(2, ChronoUnit.MINUTES)); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would remove this override -- I don't like the hard coded 2 minute wait time here. We can address default times in Synq
-- darcy-framework/synq#17
Changes Unknown when pulling 27769a0 on abocz:add-openandwait-to-browser into * on darcy-framework:master*. |
Awesome, thanks! |
Add Browser#openAndWait for convenience (fixes #28)
I'm not sure what the best way to make the predetermined duration for
openAndWait(ViewUrl<T> viewUrl
configurable. Also not sure if you wanted to add javadocs here, but I gave it a shot.